Ignore layout shift when visibility:hidden becomes visible We should ignore layout shift if the object was ineligible for layout shift tracking during the previous paint invalidation. Add LayoutObject::ShouldSkipNextLayoutShiftTracking() and set it during paint invalidation if the object is ineligible for layout shift tracking. In the next paint invalidation, if the flag is true, skip layout shift tracking and reset the flag. This also applies to content-visibility:auto for which we cleared LayoutBox's previous size to prevent layout shift tracking for the next cycle. The new flag is basically equivalent to the old method, but also covers more cases, e.g. the old method would fail if the object had visual overflow because PreviousPhysicalVisualOverflowRect() was not based on PreviousSize() and empty. Bug: 1152869 Change-Id: I000489dd8093dab8d2ab3605ad4ce3bd39fd11b0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2591367 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#837627} diff --git a/layout-instability/content-visibility-auto-offscreen.html b/layout-instability/content-visibility-auto-offscreen.html index af60412..6f7af1a 100644 --- a/layout-instability/content-visibility-auto-offscreen.html +++ b/layout-instability/content-visibility-auto-offscreen.html
@@ -1,23 +1,12 @@ <!DOCTYPE html> <title>Layout Instability: off-screen content-visibility:auto content</title> <link rel="help" href="https://wicg.github.io/layout-instability/" /> -<style> - #auto { - content-visibility: auto; - contain-intrinsic-size: 1px; - width: 100px; - } -</style> -<div class=auto> - <div style="width: 100px; height: 100px"></div> -</div> -<div class=auto style="position: relative; top: 100000px"> - <div style="width: 100px; height: 100px"></div> -</div> <script src="/resources/testharness.js"></script> <script src="/resources/testharnessreport.js"></script> <script src="resources/util.js"></script> <script> +// These scripts need to be before the contents because we need to ensure no +// layout shifts during page load. promise_test(async () => { const watcher = new ScoreWatcher; @@ -29,10 +18,30 @@ assert_equals(watcher.score, 0); + // This should report a layout shift as target is now visible. + target.style.top = '100100px'; + + await watcher.promise; + const expectedScore = computeExpectedScore(100 * 100, 100); + assert_equals(watcher.score, expectedScore); + + // No new layout shift should be reported when target is scrolled out of screeen. window.scrollTo(0, 0); await waitForAnimationFrames(2); - assert_equals(watcher.score, 0); + assert_equals(watcher.score, expectedScore); }, 'off-screen content-visibility:auto'); - </script> +<style> + .auto { + content-visibility: auto; + contain-intrinsic-size: 1px; + width: 100px; + } +</style> +<div class=auto> + <div style="width: 100px; height: 100px"></div> +</div> +<div id="target" class=auto style="position: relative; top: 100000px"> + <div style="width: 100px; height: 100px"></div> +</div> \ No newline at end of file diff --git a/layout-instability/content-visibility-auto-onscreen.html b/layout-instability/content-visibility-auto-onscreen.html index 4d9d06a..6a43080 100644 --- a/layout-instability/content-visibility-auto-onscreen.html +++ b/layout-instability/content-visibility-auto-onscreen.html
@@ -1,6 +1,20 @@ <!DOCTYPE html> <title>Layout Instability: on-screen content-visibility:auto content</title> <link rel="help" href="https://wicg.github.io/layout-instability/" /> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<script src="resources/util.js"></script> +<script> +// These scripts need to be before the contents because we need to ensure no +// layout shifts during page load. +promise_test(async () => { + const watcher = new ScoreWatcher; + + // Wait for the initial render to complete. + await waitForAnimationFrames(2); + assert_equals(watcher.score, 0); +}, 'on-screen content-visibility:auto'); +</script> <style> #target { content-visibility: auto; @@ -11,16 +25,3 @@ <div id=target> <div style="width: 100px; height: 100px"></div> </div> -<script src="/resources/testharness.js"></script> -<script src="/resources/testharnessreport.js"></script> -<script src="resources/util.js"></script> -<script> -promise_test(async () => { - const watcher = new ScoreWatcher; - - // Wait for the initial render to complete. - await waitForAnimationFrames(2); - assert_equals(watcher.score, 0); -}, 'on-screen content-visibility:auto'); - -</script> diff --git a/layout-instability/content-visibility-auto-resize.html b/layout-instability/content-visibility-auto-resize.html index b8de7f5..d692625 100644 --- a/layout-instability/content-visibility-auto-resize.html +++ b/layout-instability/content-visibility-auto-resize.html
@@ -1,22 +1,12 @@ <!DOCTYPE html> <title>Layout Instability: resizing content-visibility:auto content</title> <link rel="help" href="https://wicg.github.io/layout-instability/" /> -<style> - .auto { - content-visibility: auto; - contain-intrinsic-size: 10px 3000px; - width: 100px; - } - .contained { - height: 100px; - } - </style> - <div class=a><div class=contained></div></div> - <div class=a ><div class=contained></div></div> <script src="/resources/testharness.js"></script> <script src="/resources/testharnessreport.js"></script> <script src="resources/util.js"></script> <script> +// These scripts need to be before the contents because we need to ensure no +// layout shifts during page load. promise_test(async () => { const watcher = new ScoreWatcher; @@ -25,5 +15,16 @@ assert_equals(watcher.score, 0); }, 'off-screen content-visibility:auto'); - </script> +<style> + .auto { + content-visibility: auto; + contain-intrinsic-size: 10px 3000px; + width: 100px; + } + .contained { + height: 100px; + } +</style> +<div class=auto><div class=contained></div></div> +<div class=auto><div class=contained></div></div> diff --git a/layout-instability/content-visibility-hidden.html b/layout-instability/content-visibility-hidden.html index 939b1a2..db087fe 100644 --- a/layout-instability/content-visibility-hidden.html +++ b/layout-instability/content-visibility-hidden.html
@@ -1,6 +1,20 @@ <!DOCTYPE html> <title>Layout Instability: content-visibility:hidden content</title> <link rel="help" href="https://wicg.github.io/layout-instability/" /> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<script src="resources/util.js"></script> +<script> +// These scripts need to be before the contents because we need to ensure no +// layout shifts during page load. +promise_test(async () => { + const watcher = new ScoreWatcher; + + // Wait for the initial render to complete. + await waitForAnimationFrames(2); + assert_equals(watcher.score, 0); +}, 'on-screen content-visibility:auto'); +</script> <style> #target { content-visibility: hidden; @@ -11,16 +25,3 @@ <div id=target> <div style="width: 100px; height: 100px"></div> </div> -<script src="/resources/testharness.js"></script> -<script src="/resources/testharnessreport.js"></script> -<script src="resources/util.js"></script> -<script> -promise_test(async () => { - const watcher = new ScoreWatcher; - - // Wait for the initial render to complete. - await waitForAnimationFrames(2); - assert_equals(watcher.score, 0); -}, 'on-screen content-visibility:auto'); - -</script> diff --git a/layout-instability/visibility-hidden-layout-and-visible.html b/layout-instability/visibility-hidden-layout-and-visible.html new file mode 100644 index 0000000..3fea6bb --- /dev/null +++ b/layout-instability/visibility-hidden-layout-and-visible.html
@@ -0,0 +1,33 @@ +<!DOCTYPE html> +<title>Layout Instability: visibility:hidden change with layout</title> +<link rel="help" href="https://wicg.github.io/layout-instability/" /> +<div id="target" style="position: absolute; top: 0; width: 200px; height: 200px; visibility: hidden"></div> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<script src="resources/util.js"></script> +<script> + +promise_test(async () => { + const watcher = new ScoreWatcher; + + // Wait for the initial render to complete. + await waitForAnimationFrames(2); + + // Shift target, for which no shift should be reported because it's hidden. + target.style.top = '200px'; + target.style.visibility = 'visible'; + + await waitForAnimationFrames(2); + // No shift should be reported. + assert_equals(watcher.score, 0); + + // Shift again, for which shift should be reported. + target.style.top = '300px'; + + await watcher.promise; + const expectedScore = computeExpectedScore(200 * (200 + 100), 100); + assert_equals(watcher.score, expectedScore); + +}, 'visibility:hidden change with layout'); + +</script> 